Skip to content

[9.0][MIG][auth_signup_verify_email] Migration. - #436

Merged
moylop260 merged 10 commits into
OCA:9.0from
Tecnativa:9.0-auth_signup_verify_email-migration
May 31, 2016
Merged

[9.0][MIG][auth_signup_verify_email] Migration.#436
moylop260 merged 10 commits into
OCA:9.0from
Tecnativa:9.0-auth_signup_verify_email-migration

Conversation

@yajo

@yajo yajo commented May 26, 2016

Copy link
Copy Markdown
Member

Migrate to v9.

@Tecnativa #271.

@pedrobaeza pedrobaeza mentioned this pull request May 26, 2016
59 tasks
@pedrobaeza
pedrobaeza force-pushed the 9.0-auth_signup_verify_email-migration branch from 5b18bd0 to bdbe60c Compare May 27, 2016 07:49
@coveralls

coveralls commented May 27, 2016

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-1.8%) to 71.267% when pulling bdbe60c on Tecnativa:9.0-auth_signup_verify_email-migration into c6deb6e on OCA:9.0.

@pedrobaeza
pedrobaeza force-pushed the 9.0-auth_signup_verify_email-migration branch from bdbe60c to 33a156d Compare May 27, 2016 07:58
@pedrobaeza

Copy link
Copy Markdown
Member

Is there any chance to increase test coverage?

@coveralls

coveralls commented May 27, 2016

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-1.8%) to 71.267% when pulling 33a156d on Tecnativa:9.0-auth_signup_verify_email-migration into c6deb6e on OCA:9.0.

@yajo

yajo commented May 27, 2016

Copy link
Copy Markdown
Member Author

Let me try

@yajo
yajo force-pushed the 9.0-auth_signup_verify_email-migration branch from b3560a3 to f4e549f Compare May 27, 2016 11:44
@yajo
yajo force-pushed the 9.0-auth_signup_verify_email-migration branch from f4e549f to 1ce88c2 Compare May 27, 2016 11:49
@yajo

yajo commented May 27, 2016

Copy link
Copy Markdown
Member Author

It's being tested now.

Is it possible that Travis is failing because it's runing in a non-CSRF mode while this test is CSRF-enabled?

@moylop260

Copy link
Copy Markdown
Contributor

@yajo
What change we need in server to run the tests fine?

@yajo

yajo commented May 30, 2016

Copy link
Copy Markdown
Member Author

As you see in the docs, there's built in CSRF protection for v9. My tests woked fine locally, but it seems like the csrf_token field cannot be found in the result. The most probable cause I imagine is that somehow Travis' Odoo does not have that CSRF validation enabled. Is it possible? Otherwise it makes no sense that local and runbot's tests work and Travis not. If so, it should be enabled and further tests should implement it (although mine I think is the first test in OCA that acutally needs that).

@moylop260

moylop260 commented May 30, 2016

Copy link
Copy Markdown
Contributor

@yajo
I have reproduced the error locally.

I ran a travis2docker

# Create Dockerfiles
travisfile2dockerfile git@github.com:OCA/server-tools.git pull/436

# Create docker image
/tmp/script/git_github.com_oca_server-tools.git/pull_436/1ce88c2a2300653fdf1f784c088814293999d49e/3/10-build.sh --no-cache

# Create docker container
/tmp/script/git_github.com_oca_server-tools.git/pull_436/1ce88c2a2300653fdf1f784c088814293999d49e/3/20-run.sh "--name=csrf_token_error --entrypoint=bash"

# In container ran
/entrypoint.sh

After In container ran
~/odoo-9.0/odoo.py -d openerp_test -u auth_signup_verify_email --test-enable --stop-after-init

screen shot 2016-05-30 at 12 00 52 pm

I added a pdb to debug.

diff --git a/auth_signup_verify_email/tests/test_verify_email.py b/auth_signup_verify_email/tests/test_verify_email.py
index 29ce113..e3da7a1 100644
--- a/auth_signup_verify_email/tests/test_verify_email.py
+++ b/auth_signup_verify_email/tests/test_verify_email.py
@@ -13,6 +13,7 @@ class UICase(HttpCase):
         super(UICase, self).setUp()
         settings = self.env["base.config.settings"].create({})
         settings.auth_signup_uninvited = True
+        import pdb;pdb.set_trace()
         self.data = {
             "csrf_token": self.csrf_token(),
             "name": "Somebody",

In the line

server-tools/auth_signup_verify_email/tests/test_verify_email.py(37)csrf_token() <- return doc.xpath("//input[@name='csrf_token']")[0].get("value")

I used:
import lxml;lxml.html.tostring(doc)
The result was:
<html><head><title>404 Not Found</title>\n</head><body><h1>Not Found</h1>\n<p>The requested URL was not found on the server. If you entered the URL manually please check your spelling and try again.</p>\n</body></html>

I don't know how to fix it but if this information is useful for you, Could you help me to create hypothesis to check them?

@moylop260

Copy link
Copy Markdown
Contributor

In other matter:
runbot don't ran the test of this module
runbot-log

...
2016-05-27 12:01:21,282 13958 INFO 3147860-436-1ce88c-all openerp.modules.loading: loading sales_team/sales_team_demo.xml
2016-05-27 12:01:21,527 13958 INFO 3147860-436-1ce88c-all openerp.modules.module: module auth_signup_verify_email: creating or updating database tables
2016-05-27 12:01:21,528 13958 INFO 3147860-436-1ce88c-all openerp.modules.loading: loading auth_signup_verify_email/views/signup.xml
# Here should be the tests
2016-05-27 12:01:21,620 13958 INFO 3147860-436-1ce88c-all openerp.modules.module: module mail_environment: creating or updating database tables

screen shot 2016-05-30 at 12 46 48 pm

@pedrobaeza

Copy link
Copy Markdown
Member

@moylop260, OCA runbot doesn't perform any test, or are you talking about your runbot?

@moylop260

moylop260 commented May 30, 2016

Copy link
Copy Markdown
Contributor

@moylop260, OCA runbot doesn't perform any test, or are you talking about your runbot?

@pedrobaeza
Thank you
I did not know that

Otherwise it makes no sense that local and runbot's tests work and Travis not.

@yajo FYI runbot doesn't perform any test

@moylop260

Copy link
Copy Markdown
Contributor

@yajo
I have created a test-runbot using runbot_travis2docker module.

http://test.runbot.vauxoo.com/runbot/repo/git-github-com-oca-server-tools-git-2

Here you can see the error replicated.

If you give me your public ssh key I can give you ssh access to container with the error reproduced.

moylop260(at)vauxoo(dot)com

@yajo

yajo commented May 31, 2016

Copy link
Copy Markdown
Member Author

Hi, thanks for the debugging, I think I have a clue, let me see.

@yajo

yajo commented May 31, 2016

Copy link
Copy Markdown
Member Author

Test was affected by odoo/odoo#12237, it's workarounded now. Thanks!

@coveralls

coveralls commented May 31, 2016

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.9%) to 73.988% when pulling 48842f3 on Tecnativa:9.0-auth_signup_verify_email-migration into c6deb6e on OCA:9.0.

@moylop260

Copy link
Copy Markdown
Contributor

Thank you @yajo

👍

@carlosdauden

carlosdauden commented May 31, 2016

Copy link
Copy Markdown
Contributor

👍

1 similar comment
@sergio-teruel

Copy link
Copy Markdown
Contributor

👍

@yajo

yajo commented May 31, 2016

Copy link
Copy Markdown
Member Author

Ready to merge

@moylop260
moylop260 merged commit c10789a into OCA:9.0 May 31, 2016
@yajo
yajo deleted the 9.0-auth_signup_verify_email-migration branch May 31, 2016 14:43
JosDeGraeve pushed a commit to apertoso/server-tools that referenced this pull request Jun 13, 2016
* Add module auth_signup_verify_email.

* Import module following guidelines.

* README typos.

* OCA Transbot updated translations from Transifex

* Credit creator.

* author name correction

* [9.0][MIG][auth_signup_verify_email] Migration.

Migrate to v9.

* [FIX] auth_signup_verify_email: Python library requirement

* Add tests, fix xml tags, fix credits.

* Fix test.
JayVora-SerpentCS pushed a commit to JayVora-SerpentCS/server-tools that referenced this pull request Sep 12, 2016
* Add module auth_signup_verify_email.

* Import module following guidelines.

* README typos.

* OCA Transbot updated translations from Transifex

* Credit creator.

* author name correction

* [9.0][MIG][auth_signup_verify_email] Migration.

Migrate to v9.

* [FIX] auth_signup_verify_email: Python library requirement

* Add tests, fix xml tags, fix credits.

* Fix test.
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (12.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants